fix(thread-pool): report a bootloader failure through the worker's error stream - #225
Merged
Merged
Conversation
…ror stream A bootloader that threw was kept for a later submit() and delivered to the awaiters of the tasks already queued, and nowhere else. A pool driven as a set of long-lived workers reads neither channel: the HTTP server submits one internal task per worker and awaits it only for completion, so the exception was lost and the pool died with no diagnostic at all. The worker now hands the exception to zend_exception_error() before it closes the pool, which reports it the way any uncaught exception is reported — display_errors, log_errors and error_log all apply. That call adds E_DONT_BAIL, so the worker still reaches the close/reject path. The bailout branch keeps its silence: a fatal error has already printed itself on the way there, and exit() is not an error.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A
ThreadPoolbootloader that throws had two ways out of the worker, and both are private to whoever submits tasks: the message is kept for the nextsubmit(), and the tasks already queued are rejected with the exception. A pool driven as a set of long-lived workers reads neither. The HTTP server submits one internal task per worker and awaits it only for completion, so the exception was dropped and the process exited 0 with nothing on stdout, nothing inerror_log, and no worker ever having served.The worker now hands the exception to
zend_exception_error()before it closes the pool. That is the same call the engine makes for an uncaught exception at the end of a request, sodisplay_errors,log_errors,error_loganderror_reportingall apply, and the output reads as usual:zend_exception_error()addsE_DONT_BAIL, so the worker still reaches the close-and-reject path and every existing consumer sees what it saw before.set_exception_handler()is not invoked: the exception is reported, not dispatched.The bailout branch (
exit()in the bootloader, or a fatal error) keeps its silence — a fatal error has already printed itself on the way to the bailout, andexit()is not an error.Tests
080-bootloader_exception_reported.phptis new: a pool whose bootloader throws, with nothing submitted at all, must still produce the fatal.063-bootloader_exception.phptkeeps its assertion that the message reaches the awaiter and now also expects the report.Locally:
ext/async, 1230 tests, 3 failures —curl/063,curl/064,io/082. None of them creates aThreadPool, so this change cannot run in them.